Skip to content

Conversation

@arubdesu
Copy link
Contributor

Add if with return for case, tests that cover new behavior, assurance in test that only this path will lead to this behavior

We can definitely discuss how I separated out a new MockPref (I was following a colleagues guidance), it would make sense for the whole test struct to ride alongside the implementation in the other... package? Maybe? Anyway pardon I hacked it in, I can't even get a test build spit out at the moment because no homebrew/I've forgotten how to luggage 😅

Add if with return for case, tests that cover new behavior, assurance in test that only this path will lead to this behavior
@arubdesu
Copy link
Contributor Author

Oh, and as a reminder this is because I used an HTTP api gateway to 'forward' escrow's into the network our CryptServer is on, which doesn't allow the trailing slash and 'default' is the AWS... default naming of the stage we deployed.

@grahamgilbert
Copy link
Owner

I would prefer to not hard code specific implementation details for your environment. Is there any reason you cannot set your server url to what you need?

@arubdesu
Copy link
Contributor Author

It's certainly not the communities problem that our infrastructure implementation is weird, but linux and windows 'agent' interfaces are using this same URL so migrating has more friction/requires more coordination for something as vital as FDE escrow. I'm honestly not sure if the other API gateway options (lambda-less and REST-ful) would even fix the incompatibility with trailing slash.
Currently (and historically, which is why we had been maintaining a fork,) the checkin script breaks our usage because it defaults to appending/ensuring the trailing slash url format - even if it may be considered more 'correct' (and admittedly is definitely a single standard that was always supported) it's not impossible another environment will ever want to support something like this.

A flag to not validate/enforce trailing slash would work for me just the same, should I add a pref or something instead?

@grahamgilbert
Copy link
Owner

I think the pref to not enforce the trailing slash makes sense

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants